Add local-first Smart Notes dictation#583
Conversation
|
The PR Policy check is blocking this PR because required template information is missing. Please update the PR description with:
Visual files detected:
Screenshots or video are required for UI, UX, settings, onboarding, overlay, menu bar, or visual behavior changes. If this PR has no visual changes, check the no-visual-change box in the template. If this remains incomplete for 48 hours after opening, the PR may be closed. |
There was a problem hiding this comment.
Pull request overview
This PR adds a new local-first “Smart Notes” dictation flow that can be triggered globally, persists notes as Markdown under Documents/FluidVoice Notes, and optionally enriches them via the configured AI provider, with an in-app browser to view/manage notes.
Changes:
- Added
SmartNotesStorefor capture, atomic Markdown persistence, reload, and AI enrichment updates. - Integrated a configurable global “Smart Notes” hotkey and a dedicated output route that bypasses typing/clipboard/history.
- Added a Smart Notes UI (browser/detail) plus focused store/parser tests.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| Tests/FluidDictationIntegrationTests/SmartNotesStoreTests.swift | Adds targeted tests for raw capture, reload, AI response parsing, and in-place enrichment. |
| Sources/Fluid/UI/SmartNotesView.swift | Introduces a Smart Notes browser/detail UI with refresh/reveal/delete actions and AI toggle. |
| Sources/Fluid/UI/SettingsView.swift | Adds Smart Notes shortcut configuration UI using existing shortcut recording patterns. |
| Sources/Fluid/Services/NotificationService.swift | Adds Smart Notes AI-fallback notification plumbing. |
| Sources/Fluid/Services/GlobalHotkeyManager.swift | Adds Smart Notes as a new hotkey-triggerable recording mode across hold/automatic/toggle modes. |
| Sources/Fluid/Persistence/SmartNotesStore.swift | Implements the Smart Notes persistence/enrichment store and Markdown file format. |
| Sources/Fluid/Persistence/SettingsStore.swift | Adds persisted settings for Smart Notes shortcut enablement, shortcut binding, and AI enrichment toggle. |
| Sources/Fluid/ContentView.swift | Wires Smart Notes end-to-end: sidebar, settings bindings, hotkey callbacks, and capture/enrichment stop-path routing. |
| Fluid.xcodeproj/project.pbxproj | Registers the new SmartNotesStoreTests file with the integration test target. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| case .denied: | ||
| break |
| let content = String(contents[closingRange.upperBound...]) | ||
| let marker = "<!-- fluidvoice:body -->\n" | ||
| let markedBody = content.components(separatedBy: marker).last ?? content | ||
| let heading = "# \(title)\n\n" | ||
| let body = markedBody.hasPrefix(heading) ? String(markedBody.dropFirst(heading.count)) : markedBody |
| private static func fileName(for date: Date, id: UUID) -> String { | ||
| "\(self.fileNameDate.string(from: date))-\(id.uuidString.prefix(8).lowercased()).md" | ||
| } |
| /// Smart Notes is opt-in so existing shortcut configurations cannot gain a collision on upgrade. | ||
| var smartNotesShortcutEnabled: Bool { | ||
| get { | ||
| let value = self.defaults.object(forKey: Keys.smartNotesShortcutEnabled) | ||
| return value as? Bool ?? false |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 39a6e28cf0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| case .toggle: | ||
| if self.asrService.isRunning { | ||
| if self.isSmartNotesRecordingProvider?() ?? false { | ||
| self.stopRecordingIfNeeded() |
There was a problem hiding this comment.
Debounce Smart Notes toggle key repeats
When Smart Notes is assigned to a normal key chord in Toggle mode, holding the key down generates repeated keyDown events. This branch does not check keyboardEventAutorepeat or latch isSmartNotesKeyPressed, so repeats can invoke triggerSmartNotes multiple times; once ASR starts, the next repeat takes the stop path and can save a truncated note. Please ignore autorepeat/debounce this toggle branch.
Useful? React with 👍 / 👎.
Greptile SummaryThis PR adds a local-first Smart Notes feature: a global hotkey captures voice dictation directly into Markdown files under
|
| Text(note.body) | ||
| .font(.body) | ||
| .lineSpacing(5) | ||
| .textSelection(.enabled) | ||
| .frame(maxWidth: .infinity, alignment: .leading) |
There was a problem hiding this comment.
Markdown body rendered as raw text —
Text(someStringVariable) in SwiftUI does not interpret Markdown; the initializer that accepts a String value skips the Markdown parser. An AI-enhanced note with body `"- Confirm build
- Notify support"
will display the literal hyphens and newlines rather than a rendered list. UsingAttributedString(markdown:)` renders the Markdown correctly.
| Text(note.body) | |
| .font(.body) | |
| .lineSpacing(5) | |
| .textSelection(.enabled) | |
| .frame(maxWidth: .infinity, alignment: .leading) | |
| Group { | |
| if let attributed = try? AttributedString(markdown: note.body, options: .init(interpretedSyntax: .inlineOnlyPreservingWhitespace)) { | |
| Text(attributed) | |
| } else { | |
| Text(note.body) | |
| } | |
| } | |
| .font(.body) | |
| .lineSpacing(5) | |
| .textSelection(.enabled) | |
| .frame(maxWidth: .infinity, alignment: .leading) |
Prompt To Fix With AI
This is a comment left during a code review.
Path: Sources/Fluid/UI/SmartNotesView.swift
Line: 169-173
Comment:
Markdown body rendered as raw text — `Text(someStringVariable)` in SwiftUI does not interpret Markdown; the initializer that accepts a `String` value skips the Markdown parser. An AI-enhanced note with body `"- Confirm build
- Notify support"` will display the literal hyphens and newlines rather than a rendered list. Using `AttributedString(markdown:)` renders the Markdown correctly.
```suggestion
Group {
if let attributed = try? AttributedString(markdown: note.body, options: .init(interpretedSyntax: .inlineOnlyPreservingWhitespace)) {
Text(attributed)
} else {
Text(note.body)
}
}
.font(.body)
.lineSpacing(5)
.textSelection(.enabled)
.frame(maxWidth: .infinity, alignment: .leading)
```
How can I resolve this? If you propose a fix, please make it concise.| Button("Delete Note", role: .destructive) { | ||
| guard let note = self.notePendingDeletion else { return } | ||
| try? self.store.delete(note) | ||
| self.notePendingDeletion = nil | ||
| self.selectedNoteID = self.store.notes.first?.id |
There was a problem hiding this comment.
Delete error silently swallowed
try? self.store.delete(note) discards any FileManager.removeItem error. If the deletion fails (e.g., permissions, file in use), store.reload() is still called inside delete() and re-reads the file from disk, so the note silently reappears in the list with no feedback. The user sees the confirmation sheet close but the note stays, which is confusing. Consider at minimum logging the error or showing a brief alert.
Prompt To Fix With AI
This is a comment left during a code review.
Path: Sources/Fluid/UI/SmartNotesView.swift
Line: 46-50
Comment:
**Delete error silently swallowed**
`try? self.store.delete(note)` discards any `FileManager.removeItem` error. If the deletion fails (e.g., permissions, file in use), `store.reload()` is still called inside `delete()` and re-reads the file from disk, so the note silently reappears in the list with no feedback. The user sees the confirmation sheet close but the note stays, which is confusing. Consider at minimum logging the error or showing a brief alert.
How can I resolve this? If you propose a fix, please make it concise.| case .smartNotes: | ||
| self.smartNotesHotkeyShortcut = shortcut | ||
| SettingsStore.shared.smartNotesHotkeyShortcut = shortcut | ||
| self.hotkeyManager?.updateSmartNotesShortcut(shortcut) |
There was a problem hiding this comment.
Double manager update on shortcut assignment
applyShortcut(.smartNotes) both writes to SettingsStore and calls hotkeyManager?.updateSmartNotesShortcut(shortcut) directly. The onChange(of: smartNotesHotkeyShortcut) handler (line 485-488) then fires and calls the manager a second time. The same pattern applies to setShortcutTargetEnabled(.smartNotes) (line 1181-1184) paired with its onChange → handleSmartNotesShortcutEnabledChange, which also calls updateSmartNotesShortcutEnabled twice.
The existing .pasteLast case avoids this: applyShortcut(.pasteLast) only sets state and persists to SettingsStore; the manager reads the shortcut live from SettingsStore instead of holding its own copy. Either adopt the same read-through approach for Smart Notes, or remove the direct manager calls from applyShortcut/setShortcutTargetEnabled and let onChange remain the single update path.
Prompt To Fix With AI
This is a comment left during a code review.
Path: Sources/Fluid/ContentView.swift
Line: 1130-1133
Comment:
**Double manager update on shortcut assignment**
`applyShortcut(.smartNotes)` both writes to `SettingsStore` and calls `hotkeyManager?.updateSmartNotesShortcut(shortcut)` directly. The `onChange(of: smartNotesHotkeyShortcut)` handler (line 485-488) then fires and calls the manager a second time. The same pattern applies to `setShortcutTargetEnabled(.smartNotes)` (line 1181-1184) paired with its `onChange` → `handleSmartNotesShortcutEnabledChange`, which also calls `updateSmartNotesShortcutEnabled` twice.
The existing `.pasteLast` case avoids this: `applyShortcut(.pasteLast)` only sets state and persists to `SettingsStore`; the manager reads the shortcut live from `SettingsStore` instead of holding its own copy. Either adopt the same read-through approach for Smart Notes, or remove the direct manager calls from `applyShortcut`/`setShortcutTargetEnabled` and let `onChange` remain the single update path.
How can I resolve this? If you propose a fix, please make it concise.|
Closed because this change was intended for the fork's main branch, not the upstream repository. |
Summary
Design
SmartNotesStore owns filenames, front matter, atomic writes, reloads, and enrichment updates behind one interface. Note capture is independent from AI availability and bypasses typing, clipboard, prompt-test, and transcription-history routes.
Verification